Calling all codobots 🤖

It seems to me that maybe something in this PR has introduced an unexpected behavior?

I have an OpenAPI schema definition as such:

{
  "components": {
    "schemas": {
	  "MyObject": {
		  "properties": {
		    "backtoken": {
		      "title": "Backtoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    },
		    "skiptoken": {
		      "title": "Skiptoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    }
		  },
		  "type": "object"
	  }
	}
  }
}

This is being generated with the typescript-fetch generator as:

export interface MyObject { 
    backtoken?: string; 
    skiptoken?: string;
}

Even though the openapi spec has an explicit mention of nullable: true.

Shouldn't it still honor that nullable: true property?

It seems to me that maybe something in this PR has introduced an unexpected behavior?

I have an OpenAPI schema definition as such:

{
  "components": {
    "schemas": {
	  "MyObject": {
		  "properties": {
		    "backtoken": {
		      "title": "Backtoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    },
		    "skiptoken": {
		      "title": "Skiptoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    }
		  },
		  "type": "object"
	  }
	}
  }
}

This is being generated with the typescript-fetch generator as:

export interface MyObject { 
    backtoken?: string; 
    skiptoken?: string;
}

Even though the openapi spec has an explicit mention of nullable: true.

Shouldn't it still honor that nullable: true property?

This was mentioned and clarified before in this comment: #17798 (comment)

This change breaks the OpenAPI spec. There is a difference between a nullable and optional property. With the release of 7.4.0 all properties that can be null (a value) are just made optional (omitted from request/response).

@noordawod still getting back on this, reading carefully through the comments and reviews.

Your sentencte

The last line in the example covers the situation when you may pass in a value that is either null, undefined, or indeed a string. If you restrict it only to string | null, then undefined won't be a plausible value, which it should.

regarding this line

nullableProperty?: string,

I guess you're seeing the "pass in" from an API perspective, right? null, undefined or a string coming in from outside and is beeing parsed.

Seeing it the other way around, wanting to be able to make the resulting json in a request differentiate between null, undefined and a string is not possible with this solution, since typescript won't let you write nullableProperty = null;. Is that intended? I think it should be possible to assign null to a property defined as nullable in the openapi spec.

Other generators like sping, support differentiating between undefined, null and a string value. How would I achieve this by using the typescript-fetch-operator after this fix?

It appears that this PR has broken nullable types. null and undefined are two different things in TS.

Take the instance where we do a PATCH request and I want to partially update an entity. That means I would be able to pass in a partial object with subset of the fields.

Here's a definition of what a patch could look like, you can optionally pass any of the parameters. In this case, the subTitle field is also nullable, which would make it null in the database if I wanted to erase it:

export interface ArticlePatch {
    title?: string;
    subTitle?: string | null;
}

Scenario 1: Patching title only, I don't pass in subTitle

PATCH /articles/1
{
  "title": "My title",
}

Scenario 2: Patching subTitle only, making it null. Now I can't do this, because the types are broken:

PATCH /articles/1
{
  "subTitle": null,
}

subTitle is nullable: true in swagger.json:

{
  "subTitle": {
    "type": "string",
    "nullable": true
  }
}

In previous versions 7.3.0 and down the output was this:

export interface ArticlePatch {
    title?: string;
    subTitle?: string | null;
}

Since this PR has been merged:

export interface ArticlePatch {
    title?: string;
    subTitle?: string; // No longer nullable
}

EDIT:

I have raised a PR to address this and it has been merged:
#18887